:seedling: Add e2e regression test for cross-CE collision protection by perdasilva · Pull Request #2783 · operator-framework/operator-controller

perdasilva · 2026-06-25T10:06:39Z

Summary

Add e2e tests verifying that a second ClusterExtension targeting an already-installed bundle cannot take over resources from the original owner, even with a higher-revision ClusterObjectSet.
Use explicit CE names (ce-orig-${SCENARIO_ID}, ce-dup-${SCENARIO_ID}) in collision scenarios so every assertion unambiguously identifies which ClusterExtension it checks.
Auto-track every applied ClusterExtension for cleanup in ResourceIsApplied, eliminating the manual TrackCurrentClusterExtensionForCleanup step and the previousClusterExtensionName / ${PREV_NAME} machinery.
Add named CE condition steps (NamedClusterExtensionReportsCondition, NamedClusterExtensionReportsConditionWithMessageFragment) so step descriptions explicitly identify which CE is being asserted.
Add ClusterExtensionOwnsClusterObjectSets step to verify the number of ClusterObjectSets owned by a named CE.
Extract messageFragmentComparison helper to deduplicate the fragment-matching closure across condition steps.
Preserve unresolved template variables as literal ${KEY} instead of silently substituting empty string.

Test plan

go vet passes
New scenarios pass against a kind cluster with BoxcutterRuntime and DeploymentConfig enabled
CI e2e suite passes

🤖 Generated with Claude Code

netlify · 2026-06-25T10:06:45Z

✅ Deploy Preview for olmv1 ready!

Name	Link
🔨 Latest commit	`f79695c`
🔍 Latest deploy log	https://app.netlify.com/projects/olmv1/deploys/6a3e93954db5c100087e1210
😎 Deploy Preview	https://deploy-preview-2783--olmv1.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
🤖 Make changes	Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

openshift-ci · 2026-06-25T10:06:45Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign grokspawn for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copilot

Pull request overview

Adds an end-to-end regression scenario to ensure cross-ClusterExtension “collision protection” works (a second ClusterExtension targeting an already-installed bundle cannot take over resources), and extends the godog step library to make multi-CE assertions readable and robust.

Changes:

Adds a new e2e scenario covering duplicate ClusterExtension installs and verifies original ownership is retained.
Extends step definitions with named ClusterExtension condition assertions, a ClusterObjectSet ownership-count assertion, and a ${PREV_NAME} template variable to reference a previously-applied CE.
Refactors message-fragment matching into a shared helper and changes template expansion to preserve unknown ${KEY} variables literally.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
test/e2e/steps/steps.go	Adds named CE steps, COS ownership-count step, `${PREV_NAME}` support, message-fragment helper, and preserves unresolved template variables.
test/e2e/steps/hooks.go	Extends scenario context to track `previousClusterExtensionName`.
test/e2e/features/update.feature	Adds a new regression scenario validating cross-CE collision protection behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

codecov · 2026-06-25T10:36:35Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.50%. Comparing base (c66f3af) to head (f79695c).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2783      +/-   ##
==========================================
+ Coverage   70.40%   70.50%   +0.09%     
==========================================
  Files         143      143              
  Lines       10617    10617              
==========================================
+ Hits         7475     7485      +10     
+ Misses       2580     2573       -7     
+ Partials      562      559       -3

Flag	Coverage Δ
e2e	`35.14% <ø> (-0.03%)`	⬇️
experimental-e2e	`52.57% <ø> (+0.15%)`	⬆️
unit	`59.48% <ø> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

pedjak · 2026-06-26T11:53:17Z

+    And ClusterExtension is rolled out
+    And ClusterExtension is available
+    And ClusterExtension "${NAME}" owns 1 ClusterObjectSet
+    And the current ClusterExtension is tracked for cleanup


I would perhaps replace this with:

ClusterExtension "${NAME}" is tracked for cleanup

btw, I am not sure what does it mean from user perspective and if users need to know anything about it? Is that an implementation detail?

Removed this step entirely. ResourceIsApplied now auto-tracks every applied ClusterExtension for cleanup via addedResources, so there's no need for a manual tracking step. The previousClusterExtensionName / ${PREV_NAME} machinery has also been removed.

pedjak · 2026-06-26T11:59:22Z

+              matchLabels:
+                "olm.operatorframework.io/metadata.name": ${CATALOG:test}
+      """
+    Then ClusterExtension "${NAME}" reports Progressing as True with Reason Retrying and Message includes:


I assume ${NAME}-dup should report Progressing, not the already installed CE ${NAME}?

Fixed. Both collision scenarios now use explicit names — ce-orig-${SCENARIO_ID} for the original CE and ce-dup-${SCENARIO_ID} for the duplicate — so every assertion unambiguously identifies which CE it checks. No more implicit ${NAME} context switching.

pedjak · 2026-06-26T12:00:06Z

+      """
+    Then ClusterExtension "${NAME}" reports Progressing as True with Reason Retrying and Message includes:
+      """
+      revision object collisions


can we make this message clearer to users, as we discussed in the original PR?

The test asserts a substring match against what the controller actually emits — revision object collisions is the literal message from the ClusterObjectSet reconciler (setRetryingConditions in clusterobjectset_controller.go). Changing the wording would be a controller code change rather than a test change. Happy to open a follow-up to improve the message if you have a suggestion for clearer wording.

pedjak · 2026-06-26T12:01:02Z

+      """
+      revision object collisions
+      """
+    And ClusterExtension "${PREV_NAME}" reports Installed as True


it should be actually

ClusterExtension "${NAME}" reports Installed as True

because this is the one first installed/created in this scenario?

Fixed — now explicitly ClusterExtension "ce-orig-${SCENARIO_ID}" reports Installed as True. Both CEs are referenced by their explicit names throughout the scenario.

pedjak · 2026-06-26T12:01:36Z

+      revision object collisions
+      """
+    And ClusterExtension "${PREV_NAME}" reports Installed as True
+    # Update the conflicting ClusterExtension with a deployment config env var. At the time


are we updating here ${NAME} or ${NAME}-dup?

Fixed — the YAML now explicitly says name: ce-dup-${SCENARIO_ID} and the comment reads "Update the duplicate ClusterExtension with a deployment config env var to force a second revision."

pedjak · 2026-06-26T12:02:33Z

+                "olm.operatorframework.io/metadata.name": ${CATALOG:test}
+      """
+    Then ClusterExtension "${NAME}" owns 2 ClusterObjectSets
+    And ClusterExtension "${NAME}" reports Progressing as True with Reason Retrying and Message includes:


should be ${NAME}-dup?

Fixed — now explicitly ClusterExtension "ce-dup-${SCENARIO_ID}" throughout.

pedjak · 2026-06-26T12:03:25Z

+      """
+      revision object collisions
+      """
+    And ClusterExtension "${PREV_NAME}" reports Installed as True


how do we know that 2nd ClusterObjectSet is installed successfully?

The test intentionally verifies that the duplicate CE cannot install — both its revisions collide with the original CE's objects. The assertions confirm this:

ce-dup owns 2 ClusterObjectSets → revision 2 was created

ce-dup reports Progressing/Retrying with "revision object collisions" → both revisions are blocked

ce-orig reports Installed=True → the original CE is unaffected

I initially considered using the progression deadline timeout to verify the dup CE settles into a terminal failure state, but it started adding too much complexity and wait time for what is fundamentally a regression test. Would there be a better signal we could use here? For example, would asserting the dup stays in Retrying for 30s–60s be sufficient, or is there a cleaner way to verify the collision is persistent?

- Add e2e tests verifying that a second ClusterExtension targeting an already-installed bundle cannot take over resources from the original owner, even with a higher-revision ClusterObjectSet. - Use explicit CE names (ce-orig-${SCENARIO_ID}, ce-dup-${SCENARIO_ID}) in collision scenarios so every assertion unambiguously identifies which ClusterExtension it checks. - Auto-track every applied ClusterExtension for cleanup in ResourceIsApplied, eliminating the manual TrackCurrentClusterExtensionForCleanup step and the previousClusterExtensionName / ${PREV_NAME} machinery. - Add named CE condition steps (NamedClusterExtensionReportsCondition, NamedClusterExtensionReportsConditionWithMessageFragment) so step descriptions explicitly identify which CE is being asserted. - Add ClusterExtensionOwnsClusterObjectSets step to verify the number of ClusterObjectSets owned by a named CE. - Extract messageFragmentComparison helper to deduplicate the fragment-matching closure across condition steps. - Preserve unresolved template variables as literal ${KEY} instead of silently substituting empty string. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.

 	if res.GetKind() == "ClusterExtension" {
 		sc.clusterExtensionName = res.GetName()
+		sc.addedResources = append(sc.addedResources, resource{name: res.GetName(), kind: "clusterextension"})
 	} else if res.GetKind() == "ClusterObjectSet" {


Copilot AI review requested due to automatic review settings June 25, 2026 10:06

openshift-ci Bot requested review from oceanc80 and tmshort June 25, 2026 10:06

Copilot started reviewing on behalf of perdasilva June 25, 2026 10:07 View session

perdasilva mentioned this pull request Jun 25, 2026

🌱 Upgrade boxcutter to v0.14.0 and adapt sibling owners API #2764

Merged

4 tasks

Copilot AI reviewed Jun 25, 2026

View reviewed changes

Comment thread test/e2e/steps/steps.go

perdasilva force-pushed the e2e-collision-regression-test branch from 943273a to a29cdef Compare June 25, 2026 10:14

pedjak reviewed Jun 26, 2026

View reviewed changes

Copilot AI review requested due to automatic review settings June 26, 2026 14:58

perdasilva force-pushed the e2e-collision-regression-test branch from a29cdef to f79695c Compare June 26, 2026 14:58

Copilot started reviewing on behalf of perdasilva June 26, 2026 14:58 View session

Copilot AI reviewed Jun 26, 2026

View reviewed changes

Uh oh!

Conversation

perdasilva commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

netlify Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for olmv1 ready!

Uh oh!

openshift-ci Bot commented Jun 25, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

codecov Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

perdasilva commented Jun 25, 2026 •

edited

Loading

netlify Bot commented Jun 25, 2026 •

edited

Loading

codecov Bot commented Jun 25, 2026 •

edited

Loading